fix(serial) keep serial console mux closed when not needed#1421
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
dseven
force-pushed
the
serial-handling-conflict
branch
from
cb1e6ba to
80c5a5e
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 80c5a5e. Configure here.
| if consoleBroker != nil { | ||
| consoleBroker.Close() | ||
| consoleBroker = nil | ||
| } |
There was a problem hiding this comment.
TOCTOU race on nullable globals causes potential panic
Medium Severity
reopenSerialPort now sets serialMux and consoleBroker to nil, making them nullable during normal operation. Previously they were always non-nil after init. The handleSerialChannel OnMessage callback (running on a WebRTC goroutine) reads these unsynchronized globals — the nil check at the top doesn't protect against a concurrent rpcSetActiveExtension call setting them to nil between the check and the subsequent serialMux.Enqueue(...) or consoleBroker.Enqueue(...), causing a nil pointer dereference panic. The same race applies to sendCustomCommand.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 80c5a5e. Configure here.
Only open the serial console mux and broker when the extension is actually in use, otherwise the mux gets in a fight with the ATX or DC control extension in reading from the serial port. Closes: jetkvm#1420
dseven
force-pushed
the
serial-handling-conflict
branch
from
80c5a5e to
c0bcc78
Compare
|
Need some close eyes on this. I'm far from proficient in Go, RPC, etc. Maybe there's a better solution (should the mux recognise that the ATX or DC extension is loaded, and send the data there instead of the console broker??), or maybe I'm barking up the wrong tree entirely 😉 tag @adamshiervani |
1 participant
Only open the serial console mux and broker when the extension is actually in use, otherwise the mux gets in a fight with the ATX or DC control extension in reading from the serial port.
Closes: #1420
Note
Medium Risk
Changes serial port lifecycle and goroutine shutdown behavior across extensions, which could affect serial availability or introduce regressions if close/reopen ordering is wrong.
Overview
Ensures the serial console’s
ConsoleBroker/SerialMuxonly run when theserial-consoleextension is active, and are torn down when switching to other extensions to avoid competing reads on the UART.Extension switching (
rpcSetActiveExtension) and serial initialization now explicitly mount/unmount the serial console, withreopenSerialPortclosing and clearing the mux/broker before reopening the port.Makes
ConsoleBroker.Close()andSerialMux.Close()idempotent (safe to call multiple times) to reduce panics during repeated mount/unmount cycles.Reviewed by Cursor Bugbot for commit c0bcc78. Bugbot is set up for automated code reviews on this repo. Configure here.